Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[full-ci] Add keyboard selection / navigation #7153

Merged
merged 31 commits into from
Jul 8, 2022
Merged

[full-ci] Add keyboard selection / navigation #7153

merged 31 commits into from
Jul 8, 2022

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Jun 21, 2022 •
edited
Loading

Description

See #6029

Needs:
owncloud/owncloud-design-system#2218

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Jun 21, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lookacat lookacat force-pushed the keyboard-navigation-selection branch from d766c49 to d917c35 Compare June 21, 2022 14:14
@lookacat lookacat changed the title WIP selection via keyboard Add keyboard selection / navigation Jun 22, 2022
@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 75d91c7 to a9904f6 Compare June 22, 2022 09:06
@lookacat lookacat marked this pull request as ready for review June 23, 2022 13:22
@JammingBen
Copy link
Collaborator

Issues I've found while testing:

  • When navigating via arrow keys and exceeding the list (e.g. when my selection is on the last resource, and I keep pressing the down key), then the selection vanishes, but the outline still persists on the last element. When clicking up again, the resource above gets selected:
Bildschirmaufnahme.2022-06-24.um.10.53.44.mov
  • When selecting multiple resources via "shift+mouseclick" that are above the first selection, the selected row (outline) does not jump to the last clicked element (I am shift-clicking on "222222.md" in the clip).
Bildschirmaufnahme.2022-06-24.um.10.56.12.mov

@lookacat lookacat force-pushed the keyboard-navigation-selection branch from f9ffa27 to c238a71 Compare June 24, 2022 09:07
@ownclouders
Copy link
Contributor

ownclouders commented Jun 27, 2022 •
edited
Loading

Results for e2e-tests oC10 https://drone.owncloud.com/owncloud/web/26568/11/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/26568/tracing/file-alice-2022-6-30-03-14-51.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/26568/tracing/file-brian-2022-6-30-03-14-57.zip

@lookacat lookacat changed the title Add keyboard selection / navigation [full-ci] Add keyboard selection / navigation Jun 27, 2022
@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 38c586c to 2313218 Compare June 27, 2022 10:01
@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 2313218 to da6bd05 Compare June 27, 2022 10:30
@lookacat lookacat force-pushed the keyboard-navigation-selection branch 2 times, most recently from 92d653a to 547aab5 Compare June 29, 2022 11:28
@tbsbdr tbsbdr self-requested a review June 29, 2022 11:31
tbsbdr
tbsbdr previously requested changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@tbsbdr tbsbdr left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uha, this adds so much usability to ownCloud web 💌

I have one thing I want to request to change:

Scroll with selected focus:

Current behavior:

  1. upload ~100 items in one folder
  2. select the first item
  3. hit ⬇️ about 20 times
  4. focused item is not visible in the viewport
screenie_Brave.Browser_2022-06-29_at_13-30-40-converted.mp4

Expected behavior

  1. upload ~100 items in one folder
  2. select the first item
  3. hit ⬇️ about 20 times
  4. focused item is visible in the viewport: user always sees the current focused item
screenie_Brave.Browser_2022-06-29_at_13-27-08-converted.mp4

@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 547aab5 to 8cf0c19 Compare June 29, 2022 11:56
@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 9b531ca to 5ef4e4b Compare June 30, 2022 10:19
@lookacat
Add space, escape keyboard actions
c578699
@lookacat
Add select all shortcut
6a3b9cf
@lookacat
Refactor/slim code
a0657f1
@lookacat
Add latest selection checkbox outline, fix toggle multiple select issue
b3eb6a4
@lookacat
Add ctrl + left click selection
03dffbb
@lookacat
Add shift + click selection
23ab860
@lookacat
Add keyboard up/down navigation
6706075
@lookacat
Fix navigation deselects first and last row bug
9096d04
@lookacat
Fix shift+click doesnt set latest selected item
58454c1
@lookacat
Bump ODS, Address PR issues
cf34fc3
@lookacat
Fix ODS not updated, fix row click bug undefined error
626a4a1
@lookacat
Fix focus sidebar on selection killing keyboard shortcuts
9bc4379
@lookacat
Fix visibility observer
f6963df
@lookacat
Remove wait for focus
fa53552
@lookacat
Move keyboard actions into views
e03d594
@lookacat
Add/update changelog, Resolve PR issues
59f44d8
@lookacat
Fix broken row click, Address PR issues
413b322
@lookacat
Make focus via keyboard tab work
61e37a9
@lookacat
Fix scroll to resource
3d1ede8
@lookacat lookacat force-pushed the keyboard-navigation-selection branch from 7d6fce5 to 3d1ede8 Compare July 8, 2022 09:14
@sonarcloud
Copy link

sonarcloud bot commented Jul 8, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

16.7% 16.7% Coverage
0.0% 0.0% Duplication

kulmann
kulmann approved these changes Jul 8, 2022
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@lookacat lookacat dismissed stale reviews from pascalwengerter, tbsbdr, and JammingBen July 8, 2022 09:57

Outdated

@lookacat lookacat merged commit f078d51 into master Jul 8, 2022
@delete-merged-branch delete-merged-branch bot deleted the keyboard-navigation-selection branch July 8, 2022 09:58
@kulmann kulmann mentioned this pull request Jul 8, 2022
Closed
3 tasks
@micbar micbar mentioned this pull request Jul 19, 2022
Closed
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

@JammingBen JammingBen JammingBen left review comments

@tbsbdr tbsbdr tbsbdr left review comments

@fschade fschade Awaiting requested review from fschade

@dschmidt dschmidt Awaiting requested review from dschmidt

@pascalwengerter pascalwengerter Awaiting requested review from pascalwengerter

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

efficient selection of multiple files
6 participants
@lookacat @JammingBen @ownclouders @kulmann @pascalwengerter @tbsbdr